1084 qiime map #1219

josenavas · 2015-05-28T23:38:29Z

Built on top of #1207

This PR adds the ability to upload a QIIME mapping file trough the interface. When uploading a QIIME mapping file, a SampleTemplate and a PrepTemplate is created.

coveralls · 2015-05-29T00:03:17Z

Changes Unknown when pulling 9db35a5 on josenavas:1084-qiime-map into * on biocore:fix-1084*.

…-qiime-map

coveralls · 2015-05-29T15:19:29Z

Coverage increased (+0.05%) to 78.69% when pulling f774597 on josenavas:1084-qiime-map into 42326b0 on biocore:fix-1084.

antgonza · 2015-05-29T16:29:42Z

Works fine. Except that the validation doesn't accept comments (lines that start with #). Also, I know is not part of this issue but it will be great if you could fix the warnings when adding a template. The issue is that if you have warnings about EBI, processing, etc all of them are in a single line, could you put them in different ones (add </br>)? Gonna take a look a the code.

antgonza · 2015-05-29T16:33:53Z

qiita_db/metadata_template/util.py

Should you also check for LinkerPrimer, BarcodeSequence, and ReverseBarcodeSequence (optional)?

This is just a quick and fast check to see if the file is a mapping file. Note that templates do not have any # sign at the beginning, not even for comments. The fact that the file starts with '#SampleID' is enough to know that we are not parsing a template, but a mapping file.

antgonza · 2015-05-29T16:38:59Z

Code looks fine, only one comment. However, not sure if we should have a qiime map loading reimplementation here or use qiime. I think having it here could help us deprecate the other one for qiime 2.

ElDeveloper · 2015-05-29T16:42:33Z

qiita_pet/handlers/study_handlers/description_handlers.py

this comment is outdated.

It looks like I end up removing two lines that I shouldn't. I've re-added the lines that define the message and message level in case of success.

ElDeveloper · 2015-05-29T16:53:33Z

Code looks great, I'm gonna test the system now.

@biocore/qiita-admin would be good if others could test this.

ElDeveloper · 2015-05-29T16:56:48Z

As for having two parsers, I almost want to re-use the code in QIIME,
having multiple parsers for the same file-format is a nightmare and
prone to error. What do you guys think, I know there's the whole GPL
limitation, etc ... but then again, I think we should think this
through.

On (May-29-15| 9:38), Antonio Gonzalez wrote:

Code looks fine, only one comment. However, not sure if we should have a qiime map loading reimplementation here or use qiime. I think having it here could help us deprecate the other one for qiime 2.

Reply to this email directly or view it on GitHub:
#1219 (comment)

josenavas · 2015-05-29T17:00:48Z

do you want to have a call about the 2 parsers thing? I think it will be faster than discussing over here.

ElDeveloper · 2015-05-29T17:03:23Z

Sure, anyone else who would like to join?

On (May-29-15|10:00), josenavas wrote:

do you want to have a call about the 2 parsers thing? I think it will be faster than discussing over here.

Reply to this email directly or view it on GitHub:
#1219 (comment)

…ReverseLinkerPrimer

josenavas · 2015-05-29T23:07:04Z

@ElDeveloper @antgonza this should be ready to review!

adamrp · 2015-05-29T23:17:44Z

qiita_pet/handlers/study_handlers/description_handlers.py

I like the escalation from "looks like" to "is" 😆

hahaha, yeah it was way shorter....

ElDeveloper · 2015-05-30T00:20:02Z

qiita_ware/metadata_pipeline.py

Can you add this exception to this try/except.

ElDeveloper · 2015-05-30T00:26:37Z

PS - You also have some PEP8 failures:

qiita_db/metadata_template/util.py:400:13: E731 do not assign a lambda expression, use a def
qiita_db/metadata_template/util.py:403:13: E731 do not assign a lambda expression, use a def
qiita_db/metadata_template/util.py:407:13: E731 do not assign a lambda expression, use a def
qiita_db/metadata_template/util.py:410:13: E731 do not assign a lambda expression, use a def

josenavas · 2015-05-30T00:54:04Z

Thanks @ElDeveloper . I don't understand why the flake8 in travis is different from the one on my machine... I think the one in conda has something different.

ElDeveloper · 2015-05-30T01:03:43Z

Thanks @josenavas. Yeah, conda is sometimes a bit outdated compared to the pypi packages ...

ElDeveloper · 2015-05-30T01:04:24Z

I hereby officially declare this as 👍! If someone else wants to go ahead and merge, that would be awesome.

antgonza · 2015-05-30T01:10:31Z

In nomine Patris, et Filii, et Spiritus Sancti. Amen

1084 qiime map

coveralls · 2015-05-30T01:15:24Z

Coverage increased (+0.01%) to 78.65% when pulling 8b6a73e on josenavas:1084-qiime-map into 42326b0 on biocore:fix-1084.

ElDeveloper · 2015-05-30T01:23:38Z

josenavas added 10 commits May 25, 2015 22:03

Creating looks like qiime map function

b76b85f

Adding tests

4eb1cbc

Making load_template_to_dataframe to parse qiime mapping files

2fbc2ee

Adding test for qiime parsing

8d0120a

Adding metadata pipeline

4b0816f

Adding create_templates_from_qiime_mapping_file functionality

233f3b7

Adding test

360f7a7

Merge branch '1084-qiita-pet' into 1084-qiime-map

bc6943f

Moving code to handlers instead that in the template

94560b1

Modifying interface so we can upload a QIIME mapping file

9db35a5

Merge branch 'fix-1084' of https://github.com/biocore/qiita into 1084…

f774597

…-qiime-map

josenavas added this to the Alpha 0.2 milestone May 29, 2015

josenavas added GUI priority: high method addition labels May 29, 2015

josenavas mentioned this pull request May 29, 2015

1084 changelog #1221

Merged

antgonza reviewed May 29, 2015
View reviewed changes

ElDeveloper reviewed May 29, 2015
View reviewed changes

josenavas added 3 commits May 29, 2015 14:05

Adding QIIME parser for mapping files and fix load_template_to_datafrme

555ce64

Making error messages prettier

d9b9bc4

Adding check for missing QIIME mapping columns plus specially handle …

df108b9

…ReverseLinkerPrimer

adamrp reviewed May 29, 2015
View reviewed changes

Addressing @adamrp's comments

3995ae8

ElDeveloper reviewed May 30, 2015
View reviewed changes

Solving comments and pep8

8b6a73e

antgonza added a commit that referenced this pull request May 30, 2015

Merge pull request #1219 from josenavas/1084-qiime-map

3ac6ab9

1084 qiime map

antgonza merged commit 3ac6ab9 into qiita-spots:fix-1084 May 30, 2015

1084 qiime map #1219

1084 qiime map #1219

Uh oh!

Conversation

josenavas commented May 28, 2015

Uh oh!

coveralls commented May 29, 2015

Uh oh!

coveralls commented May 29, 2015

Uh oh!

antgonza commented May 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antgonza commented May 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ElDeveloper commented May 29, 2015

Uh oh!

ElDeveloper commented May 29, 2015

Uh oh!

josenavas commented May 29, 2015

Uh oh!

ElDeveloper commented May 29, 2015

Uh oh!

josenavas commented May 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ElDeveloper commented May 30, 2015

Uh oh!

josenavas commented May 30, 2015

Uh oh!

ElDeveloper commented May 30, 2015

Uh oh!

ElDeveloper commented May 30, 2015

Uh oh!

antgonza commented May 30, 2015

Uh oh!

coveralls commented May 30, 2015

Uh oh!

ElDeveloper commented May 30, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants